LTI date updates by ascholerChemeketa · Pull Request #1342 · RunestoneInteractive/rs

ascholerChemeketa · 2026-08-04T00:33:14Z

Implements changes proposed in Discord.

Maybe wait another day or two to see if anyone has strong opinions. Posting now to see what the review bot says.

Copilot

Pull request overview

This PR updates LTI 1.3 integration to support additional date fields (availability start/end) via custom launch parameters and deep-link resource payloads, along with a small UI visibility-mode precedence change and some HTTPS detection adjustments.

Changes:

Add LTI custom-claim helpers and propagate custom params through the LTI 1.3 launch flow to update visible_on / hidden_on.
Extend deep-link resource serialization to include available.startDateTime / available.endDateTime and format stored UTC datetimes for outbound LTI messages.
Adjust assignment visibility-mode precedence when both schedule dates are present, and expand HTTPS detection for proxied FastAPI requests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sample.env	Updates wording around `SERVER_PROTOCOL` guidance.
components/rsptx/lti1p3/pylti1p3/message_launch.py	Adds helpers to read LTI custom claim params from the JWT payload.
components/rsptx/lti1p3/pylti1p3/deep_link_resource.py	Adds `available` start/end datetime fields to deep-link resource serialization.
components/rsptx/lti1p3/pylti1p3/contrib/fastapi/request.py	Treats `x-forwarded-proto` as HTTPS for secure-request detection.
components/rsptx/auth/session.py	Changes how “production/HTTPS” is detected for cookie flags.
bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.ts	Makes `scheduled_period` take precedence when both schedule dates exist.
bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.spec.ts	Updates test expectation to match the new precedence.
bases/rsptx/admin_server_api/routers/lti1p3.py	Adds LTI datetime parse/format helpers; updates assignment date syncing and deep-link creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bnmnetp · 2026-08-04T12:51:55Z


    def set_cookie(self, response, token):
-        production = settings.server_config == "production"
+        production = settings.server_protocol.startswith("https://")


should add server_protocol to configuration/core.py

ascholerChemeketa · 2026-08-04T15:55:19Z

+        is_https = (
+            request_obj.url.scheme.lower() == "https"
+            or request_obj.headers.get("x-forwarded-proto", "").lower() == "https"
+        )


ascholerChemeketa · 2026-08-04T15:56:21Z

+        try:
+            availability_datetime = parse_lti_datetime_as_utc(
+                custom_params.get(custom_param)
+            )
+        except Exception:
+            # just ignore bad dates, could be missing, bad format, etc
+            continue
+
+        if availability_datetime != getattr(assign, assignment_field):
+            setattr(assign, assignment_field, availability_datetime)
+            updated = True


Overwrite is sometimes desired. I refined the logic to handle a Canvas special case and document choices

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

components/rsptx/lti1p3/pylti1p3/contrib/fastapi/request.py:70

X-Forwarded-Proto is treated as a raw substring match without normalizing case or handling the common comma-separated format. This can mis-detect secure requests behind some proxies (e.g. header value "HTTPS" or "https, http"), which affects downstream cookie/SameSite behavior and URL construction.

        is_https = (
            request_obj.url.scheme.lower() == "https"
            or "https" in request_obj.headers.get("x-forwarded-proto", "")
        )

bases/rsptx/admin_server_api/routers/lti1p3.py:640

Availability custom params (visible_on/hidden_on) are parsed as UTC even when the LMS sends a naive ISO datetime (no offset/Z). This is inconsistent with the due-date ingest logic a few lines above (which treats naive timestamps as course-local wall clock), and would store the wrong instant for platforms that omit timezone offsets.

        # If the LMS does not recognize the variable, it will be returned verbatim.
        # Otherewise, it should be a valid ISO datetime string or empty string.
        # Empty indicates a meaningful null value.
        try:
            availability_datetime = parse_lti_datetime_as_utc(raw_value)

…start and end set

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (5)

components/rsptx/lti1p3/pylti1p3/contrib/fastapi/request.py:70

"https" in x-forwarded-proto can yield false positives (e.g. header value "httpsomething") and doesn’t correctly handle comma-separated lists. This can misclassify requests as secure/insecure and affect cookie/redirect behavior. Consider parsing the header into tokens and checking for an exact https value.

        is_https = (
            request_obj.url.scheme.lower() == "https"
            or "https" in request_obj.headers.get("x-forwarded-proto", "").lower()
        )

bases/rsptx/admin_server_api/routers/lti1p3.py:158

parse_lti_datetime_as_utc() treats any falsy value as “missing” (if not datetime_string ...). If a non-string falsy value (e.g. 0/False) ever leaks into custom_params, this will be interpreted as a meaningful null and may clear an existing date. Since the function contract is Optional[str], it should check explicitly for None/empty-string and reject non-strings.

    if not datetime_string or datetime_string == "":
        return None

bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.spec.ts:34

This test encodes the same scheduled_period-when-visible behavior as getVisibilityMode() and will pass even though it conflicts with server-side visibility semantics (scheduled period is visible=false + both dates). If getVisibilityMode is corrected to only return scheduled_period when visible is false, this should continue to expect scheduled_hidden for visible=true + hidden_on (even if visible_on is also set).

  it("prefers scheduled_period when both visible_on and hidden_on are set, even if visible", () => {
    expect(getVisibilityMode(true, "2026-01-01T00:00:00Z", "2026-02-01T00:00:00Z")).toBe(
      "scheduled_period"
    );
  });

bases/rsptx/admin_server_api/routers/lti1p3.py:623

Typo in comment: “Otherewise” → “Otherwise”.

        # Otherewise, it should be a valid ISO datetime string or empty string.

bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.ts:15

scheduled_period on the server side only applies when visible is false (see is_assignment_visible_to_students in components/rsptx/db/crud/assignment.py:65-76). Returning scheduled_period whenever both dates are set (even when visible is true) misrepresents the persisted state and can cause UI edits to flip visible to false via getVisibilityValues("scheduled_period", ...). Gate this mode on !visible so visible=true + hidden_on continues to map to scheduled_hidden.

  if (visibleOn && hiddenOn) {
    return "scheduled_period";
  }

ascholerChemeketa · 2026-08-04T17:48:08Z

Think we may need to talk visibility values.

What is your mental model for the visibility values in relation to visible/visible_on/hidden_on?

In my head (informed by Canvas published flag), if visible is off, any available dates do not matter.

An alternate formulation would be that visible is automatically changed to true if after visible_on but before hidden_on.

This block seems to be halfway between those two.

    case "hidden":
      return { visible: false, visible_on: null, hidden_on: null };
    case "visible":
      return { visible: true, visible_on: null, hidden_on: null };
    case "scheduled_visible":
      return { visible: false, visible_on: visibleOn, hidden_on: null };
    case "scheduled_hidden":
      return { visible: true, visible_on: null, hidden_on: hiddenOn };
    case "scheduled_period":
      return { visible: false, visible_on: visibleOn, hidden_on: hiddenOn };

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

sample.env:92

The SERVER_PROTOCOL env var is still consumed by the legacy web2py app (e.g. applications/runestone/models/0.py reads it), but FastAPI cookie behavior now derives HTTPS from Settings.server_protocol. The updated comment no longer mentions web2py and could mislead operators into thinking this knob affects the FastAPI services directly.

# this is used to decide on how to set the session cookie settings.
# In production you will want to change this to https://
SERVER_PROTOCOL=http://

bases/rsptx/admin_server_api/routers/lti1p3.py:156

parse_lti_datetime_as_utc()'s docstring says unresolved LTI substitution values indicate a null, but the function currently treats any non-ISO value as an exception (and the Canvas-specific “unresolved means null” handling happens in the caller). Updating the docstring to match the actual behavior will avoid future misuse of this helper.

    """
    Parse an LTI ISO datetime and return a naive UTC datetime for storage.
    Unresolved LTI substitution values indicate that the LMS has a null (but knows about the variable).
    An unresolved parameter string indicates that the LMS does not recognize the variable. That will become an exception.
    """

ascholerChemeketa requested a review from bnmnetp as a code owner August 4, 2026 00:33

Copilot AI lite review requested due to automatic review settings August 4, 2026 00:33

Copilot started reviewing on behalf of ascholerChemeketa August 4, 2026 00:33 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 4, 2026 15:54

ascholerChemeketa force-pushed the lti-date-updates branch from 01ede22 to 10f600f Compare August 4, 2026 15:54

Copilot started reviewing on behalf of ascholerChemeketa August 4, 2026 15:55 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Andrew Scholer added 3 commits August 4, 2026 10:33

Use SERVER_PROTOCOL to identify when to set secure cookies

5cc2801

Sync LTI availability dates to assignments

938e2a4

Assignment builder: prefer scheduled_period to scheduled_hidden when …

db739b1

…start and end set

Copilot AI review requested due to automatic review settings August 4, 2026 17:33

ascholerChemeketa force-pushed the lti-date-updates branch from 10f600f to e334914 Compare August 4, 2026 17:33

Copilot started reviewing on behalf of ascholerChemeketa August 4, 2026 17:34 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

LTI1p3: sync RS availability range to LMS on initial import

7186d09

ascholerChemeketa force-pushed the lti-date-updates branch from e334914 to 7186d09 Compare August 4, 2026 17:50

Copilot AI review requested due to automatic review settings August 4, 2026 17:50

Copilot started reviewing on behalf of ascholerChemeketa August 4, 2026 17:50 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Navigation Menu

Uh oh!

Uh oh!

LTI date updates - #1342

LTI date updates#1342
ascholerChemeketa wants to merge 4 commits into
RunestoneInteractive:mainfrom
ascholerChemeketa:lti-date-updates

ascholerChemeketa commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

bnmnetp Aug 4, 2026

Uh oh!

ascholerChemeketa Aug 4, 2026

Uh oh!

ascholerChemeketa Aug 4, 2026

Uh oh!

ascholerChemeketa Aug 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

ascholerChemeketa commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

ascholerChemeketa commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

bnmnetp Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

ascholerChemeketa Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

ascholerChemeketa Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

ascholerChemeketa Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

ascholerChemeketa commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants